Skip to content

Display public ip addresses for shared network#4676

Merged
yadvr merged 2 commits intoapache:4.15from
ravening:shared-network-changes
Apr 5, 2021
Merged

Display public ip addresses for shared network#4676
yadvr merged 2 commits intoapache:4.15from
ravening:shared-network-changes

Conversation

@ravening
Copy link
Member

Description

If a vm belongs to shared network then display the list
of ip addresses available which can be used to assign for
secondary IP addresses.

Also display "Public IP addresses" tab for shared networks

List all public ip address in shared network

Screenshot 2021-02-10 at 13 16 26

Display the list of ip address while adding secondary ip instead of user input box

Screenshot 2021-02-10 at 13 17 15

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

@shwstppr shwstppr added this to the 4.16.0.0 milestone Feb 11, 2021
@weizhouapache
Copy link
Member

@ravening does this rely on some code changes ?

@ravening
Copy link
Member Author

@ravening does this rely on some code changes ?

@weizhouapache dont think so.. public ip address should be listed by default

@weizhouapache
Copy link
Member

@ravening does this rely on some code changes ?

@weizhouapache dont think so.. public ip address should be listed by default

@ravening but listing ips in shared network does not work, right ?

@ravening
Copy link
Member Author

@ravening does this rely on some code changes ?

@weizhouapache dont think so.. public ip address should be listed by default

@ravening but listing ips in shared network does not work, right ?

@weizhouapache I can display it

(local) mgt01 > list publicipaddresses listall=true fordisplay=true allocatedonly=false forvirtualnetwork=true filter=ipaddress,
{
  "count": 59,
  "publicipaddress": [

@weizhouapache
Copy link
Member

@ravening does this rely on some code changes ?

@weizhouapache dont think so.. public ip address should be listed by default

@ravening but listing ips in shared network does not work, right ?

@weizhouapache I can display it

(local) mgt01 > list publicipaddresses listall=true fordisplay=true allocatedonly=false forvirtualnetwork=true filter=ipaddress,
{
  "count": 59,
  "publicipaddress": [

@ravening ok.
I tested with 4.15.1.0, it worked.

by the way, forvirtualnetwork should be 'false', and networkid should be passed.

@yadvr
Copy link
Member

yadvr commented Mar 24, 2021

@ravening @weizhouapache does it make sense to change base to 4.15 to 4.15.1?

@yadvr yadvr changed the base branch from master to 4.15 March 24, 2021 07:06
@yadvr yadvr changed the base branch from 4.15 to master March 24, 2021 07:06
@yadvr yadvr modified the milestones: 4.16.0.0, 4.15.1.0 Mar 24, 2021
@Pearl1594
Copy link
Contributor

@ravening Could you please address the conflict. Thanks.

@shwstppr
Copy link
Contributor

ping @ravening

@ravening ravening force-pushed the shared-network-changes branch from 26fd877 to e383dd3 Compare March 31, 2021 11:02
If a vm belongs to shared network then display the list
of ip addresses available which can be used to assign for
secondary IP addresses.

Also display "Public IP addresses" tab for shared networks
@ravening ravening force-pushed the shared-network-changes branch from e383dd3 to c990f73 Compare March 31, 2021 11:03
@ravening ravening changed the base branch from master to 4.15 March 31, 2021 11:03
@Pearl1594
Copy link
Contributor

@blueorangutan ui

@blueorangutan
Copy link

@Pearl1594 a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

@blueorangutan
Copy link

UI build: ✔️
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/4676 (SL-JID-81)

onAcquireSecondaryIPAddress (record) {
if (record.nic.type === 'Shared') {
this.editNicResource = record.nic
this.editNetworkId = record.nic.networkid
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ravening I believe L466-467 need to be moved outside the if block as in a case where we have one nic on an Isolated network & one on a shared network; if you first attempt adding a secondary IP for the nic on a shared network and then do the same for the nic on Isolated network, it continues to show the shared n/ws public ips

@yadvr
Copy link
Member

yadvr commented Apr 1, 2021

ping @ravening cc @weizhouapache - any progress/update on this?

@utchoang cc @svenvogel - can you help review/test this as well?

@utchoang
Copy link

utchoang commented Apr 1, 2021

@ravening
An error occurred while navigating to the IP address, network resource page

  • Public IP resource:
    image
    image

  • Network:
    image
    image

@ravening
Copy link
Member Author

ravening commented Apr 1, 2021

@ravening
An error occurred while navigating to the IP address, network resource page

  • Public IP resource:
    image
    image
  • Network:
    image
    image

@utchoang @Pearl1594 can you guys test/review the code again?

@Pearl1594
Copy link
Contributor

@blueorangutan ui

@blueorangutan
Copy link

@Pearl1594 a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

@blueorangutan
Copy link

UI build: ✔️
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/4676 (SL-JID-85)

Copy link

@utchoang utchoang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@yadvr yadvr requested a review from Pearl1594 April 4, 2021 07:35
Copy link
Contributor

@Pearl1594 Pearl1594 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified behavior. LGTM

@yadvr yadvr merged commit c75c6ba into apache:4.15 Apr 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants